Drop Psalm in favor of PHPStan #1852
Conversation
|
No baseline please. Pick the level project follows. I don't want people getting errors they have to fix every time they touch random file. |
|
They would have to write code for the current level while the existing errors in baseline can be fixed over time. It would get harder to reach a higher level when new code bring new errors. |
|
We could argue about pros and cons of baselines, but let's have that discussion separately. We didn't use baseline with psalm, so let's not add it in PR which claims to just replace psalm. |
| - src/Command/Proxy/ConvertMappingDoctrineCommand.php | ||
| - src/Command/Proxy/EnsureProductionSettingsDoctrineCommand.php |
There was a problem hiding this comment.
Apparently, the errors in these 2 files cannot be baselined
phpstan.neon.dist
Outdated
| @@ -0,0 +1,13 @@ | |||
| parameters: | |||
| level: 8 | |||
| phpVersion: 80402 | |||
There was a problem hiding this comment.
I would prefer phpstan to infer the correct version
As per the decision we made during the hackathon. I have left many psalm-suppress annotations that seem informative in the codebase.
|
@ostrolucky I agree with @SenseException , but I want you to be happy with how this repo looks like since you're so active in it. I pushed a version with what you asked… it means we are now at PHPStan level 1 😬 |
| @@ -0,0 +1,9 @@ | |||
| parameters: | |||
| level: 1 | |||
There was a problem hiding this comment.
This is way too low compared to on what level Psalm ran.
There was a problem hiding this comment.
Yeah :( I will try to increase it slowly. Psalm was running on high level because of carefully crafted suppression rules, somebody has to put in the work to make them for phpstan. I've already run into phpstan/phpstan#12414 (this is the kind of things baselines hide)
I'll make it to lvl2 today. Let me know if you would be interested in help to update it to some other lvl.
There was a problem hiding this comment.
I'll have a look, but I won't be able to start right away. A temporary baseline would've been good in order to not lose coverage and track the progress.
Could you (or anyone else) explain the reasons why you decided to make such a replacement? |
|
Note that previously, most other Doctrine packages were running both tools, not just Psalm. This means that the first reason does not apply to this repo specifically, but consistency in tooling in the organization also helps. |
As per the decision we made during the hackathon.
I have left many
psalm-suppressannotations that seem informative in the codebase.